Skip to content

docs: architecture review — fix inaccuracies and add potential challenges#4

Merged
FisherSkyi merged 2 commits intoclaude/issue-2-20260321-1420from
copilot/sub-pr-3
Mar 21, 2026
Merged

docs: architecture review — fix inaccuracies and add potential challenges#4
FisherSkyi merged 2 commits intoclaude/issue-2-20260321-1420from
copilot/sub-pr-3

Conversation

Copy link
Copy Markdown

Copilot AI commented Mar 21, 2026

Reviewer requested identification of potential challenges and non-optimal descriptions in the proposed architecture doc.

Description fixes

  • Hono: removed "edge-ready" claim — Socket.io requires a persistent Node.js http.Server; serverless/edge deployment is ruled out
  • JWT: "stateless" applies only to access tokens; refresh tokens are Redis-stored for revocation
  • AI routing: expanded from vague "Per-SDK calls" to explicit SDK list
  • Package manager: resolved "npm or Bun" ambiguity → npm
  • Drizzle pooling: Drizzle has no built-in pool; corrected to postgres.js driver / PgBouncer
  • AI streaming: resolved "SSE or WebSocket" — SSE to requester, server relays chunks over Socket.io to room
  • Model identifiers: pinned to versioned names (claude-sonnet-4-5) instead of informal aliases

Schema fixes

  • users.avatar: NOT NULL → nullable; color gains a CHECK constraint
  • versions.action: unvalidated free text → CHECK with defined enum values
  • versions.message_id: ON DELETE CASCADEON DELETE SET NULL + snapshot TEXT column to preserve audit content on message deletion

Batch flush hardening

  • Added token budget check before provider API call (prevents cryptic context-window errors at runtime)
  • Wrapped AI call + DB write in a transaction (queued messages preserved on failure)

New section: Potential Challenges & Points to Notice (12 items)

  • Hono + Socket.io wiring (must share raw http.Server)
  • SSE → Socket.io fan-out relay design (per-token vs. buffered)
  • Context window limits during flush (token counting required)
  • Presence TTL (30 s) too close to heartbeat (20 s) → false disconnects; recommendation: 3× ratio
  • Multi-instance presence race condition on reconnect
  • Audit trail integrity when messages are hard-deleted
  • last_activity requires trigger or careful application-level UPDATE
  • OAuth/social login schema (provider, provider_id) must be planned from Phase 1
  • WebSocket events unrate-limited (REST is; Socket.io is not)
  • Shared types between src/ and server/ need npm workspaces from the start
  • No unit/integration tests until Phase 6 E2E — testing added to earlier phases
  • Free-text model column fragile against provider renames

🔒 GitHub Advanced Security automatically protects Copilot coding agent pull requests. You can protect all pull requests by enabling Advanced Security for your repositories. Learn more about Advanced Security.

…TECTURE.md

Co-authored-by: FisherSkyi <178589067+FisherSkyi@users.noreply.github.com>
Agent-Logs-Url: https://github.com/openhalibut/NetColab/sessions/45fe34ee-16bc-4100-aa3f-1c63c135686e
Copilot AI changed the title [WIP] Add architecture design for NetColab docs: architecture review — fix inaccuracies and add potential challenges Mar 21, 2026
Copilot AI requested a review from FisherSkyi March 21, 2026 14:48
@FisherSkyi FisherSkyi marked this pull request as ready for review March 21, 2026 15:02
@FisherSkyi FisherSkyi merged commit 59ffc46 into claude/issue-2-20260321-1420 Mar 21, 2026
1 of 2 checks passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants